Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Improve injectQuery path handling (fix #2422) #2435

Merged
merged 1 commit into from
Mar 15, 2021
Merged

fix: Improve injectQuery path handling (fix #2422) #2435

merged 1 commit into from
Mar 15, 2021

Conversation

GrygrFlzr
Copy link
Member

@GrygrFlzr GrygrFlzr commented Mar 8, 2021 •
edited
Loading

Fixes #2422.

This PR updates injectQuery to handle Windows paths in a saner manner.

  • As url.parse has been deprecated in Node, relative paths are now resolved using new URL(url, 'relative:///').
  • When a Windows drive letter is erroneously used as a protocol (e.g. c:), it instead resolves it using url.pathToFileURL and removes the first character (a forward slash) so that the following code in importAnalysis can properly handle it as an in-root URL:

vite/packages/vite/src/node/plugins/importAnalysis.ts

Lines 182 to 192 in 76f3535

// normalize all imports into resolved URLs
// e.g. `import 'foo'` -> `import '/@fs/.../node_modules/foo/index.js`
if (resolved.id.startsWith(root + '/')) {
// in root: infer short absolute path from root
url = resolved.id.slice(root.length)
} else if (fs.existsSync(cleanUrl(resolved.id))) {
// exists but out of root: rewrite to absolute /@fs/ paths
url = path.posix.join(FS_PREFIX + resolved.id)
} else {
url = resolved.id
}

@GrygrFlzr GrygrFlzr changed the title Improve injectQuery path handling (fix #2422) fix: Improve injectQuery path handling (fix #2422) Mar 8, 2021
This was referenced Mar 10, 2021
Closed
Closed
@antfu antfu added the p4-important Violate documented behavior or significantly improves performance (priority) label Mar 13, 2021
@lamualfa
Copy link

Sorry, Please merge this soon. I can't continue my development process because of this error: (sveltejs/kit#460)

image

@yousufiqbal
Copy link

Please merge.. waiting so long to try sveltekit.. Thanks

@yyx990803 yyx990803 merged commit a5412f8 into vitejs:main Mar 15, 2021
@lamualfa
Copy link

Yes !!!

image

@lamualfa lamualfa mentioned this pull request Mar 16, 2021
Closed
This was referenced Mar 18, 2021
Closed
Merged
@sapphi-red sapphi-red mentioned this pull request Aug 20, 2022
Merged
9 tasks
@sapphi-red sapphi-red mentioned this pull request Oct 21, 2022
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antfu antfu antfu approved these changes

Assignees
No one assigned
Labels
p4-important Violate documented behavior or significantly improves performance (priority)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Windows: Import normalization can incorrectly resolve as both in and out of root
5 participants
@GrygrFlzr @lamualfa @yousufiqbal @antfu @yyx990803